fix(pytket decoder): Panic on repeated registers in pytket decoded output by aborgna-q · Pull Request #1445 · Quantinuum/tket2

aborgna-q · 2026-03-13T14:59:15Z

Fix an edge case where we extracted a hugr from a pytket circuit, and declared multiple output ports to have the same bit register.

Also cleans up a bit of the logic that handled consumed bit wires, to avoid marking them as outdated if they can still be used by other operations.

tket/src/serialize/pytket/decoder.rs

aborgna-q · 2026-03-13T15:02:50Z

tket/src/serialize/pytket/extension/bool.rs

+            node,
+            hugr,
+            // Output bits use new registers, so don't reuse any input bits.
+            EmitCommandOptions::new().reuse_bits(|_| vec![]),


Drive-by: The classical expression emitter was re-using the input bits to the node instead of using a separate output bit register.

Yeah this looks like it might even have been the bigger fix?? Do we have a test that covers just this?

codecov · 2026-03-13T15:03:44Z

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.03%. Comparing base (2d3a5de) to head (c8ea134).

Files with missing lines	Patch %	Lines
tket/src/serialize/pytket/decoder/wires.rs	77.77%	0 Missing and 2 partials ⚠️
tket/src/serialize/pytket/extension/bool.rs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1445      +/-   ##
==========================================
+ Coverage   79.60%   80.03%   +0.43%     
==========================================
  Files         155      155              
  Lines       20335    20333       -2     
  Branches    19345    19343       -2     
==========================================
+ Hits        16187    16274      +87     
+ Misses       3188     3098      -90     
- Partials      960      961       +1

Flag	Coverage Δ
python	`93.00% <ø> (ø)`
qis-compiler	`100.00% <ø> (ø)`
rust	`79.35% <89.65%> (+0.45%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

acl-cqc

Thanks, @aborgna-q - I think there are some fairly significant improvements/fixes in, so keen to get this in :). I think a couple of comments are significant (PR title/panicking) but mostly just neatness/comprehension so I'll approve rightaway :-)

acl-cqc · 2026-03-17T10:56:46Z

tket/src/serialize/pytket/decoder/wires.rs

-            .first()
-            .into_iter()
-            .flat_map(|bit| self.bit_wires(bit));
+        let qubit_candidates = if reg_count.qubits > 0 && !qubit_args.is_empty() {


You definitely could reduce the diff - IIUC this is

let qubit_candidates = qubit_args .first() .filter(|_| reg_count.qubits > 0) // the only new bit .into_iter() .flat_map(|qb| self.qubit_wires(qb));

and same for bit_candidates. TBH I think this is simpler, and avoids Either which is a bit nasty IMHO.

tket/src/serialize/pytket/extension/bool.rs

acl-cqc · 2026-03-17T10:59:48Z

tket/src/serialize/pytket/extension/bool.rs

+        encoder.emit_node_command(
+            node,
+            hugr,
+            // Output bits use new registers, so don't reuse any input bits.


super-nit: consider assigning this to a variable first (called output_bits, thus shortening the comment?)

acl-cqc · 2026-03-17T11:02:06Z

tket/src/serialize/pytket/decoder.rs

            for qubit in encoded_info.output_qubits.iter() {
                let id = self.wire_tracker.tracked_qubit_for_register(qubit)?;
-                qubits.insert(id.clone());
+                qubits.push(id.clone());


The PR title is "Panic on repeated registers". I don't see anything that would panic if the same qubit was pushed more than once - TBH it seems to me that "repeated registers" for qubits is a fairly serious error (linearity violation), so panicking sounds like a good thing to do?

(Not sure whether "registers" means "qubits", "bits" or both - panicking on repeated bits sounds an odd thing to do if you are from any background other than pytket 😉 😁 but you might want to)

If the PR title is wrong and we don't panic, please update it ;-), maybe something about bit registers if it doesn't apply to qubits.

acl-cqc · 2026-03-17T11:02:31Z

tket/src/serialize/pytket/decoder.rs

-        // Add any additional qubits or bits we have seen, without modifying the
-        // order of the qubits already there.
+
+        // Add any additional qubits or bits we have seen but haven't added to the list.


acl-cqc · 2026-03-17T11:04:08Z

tket/src/serialize/pytket/decoder.rs

                .hugr_mut()
                .connect(wire.node(), wire.source(), node, input_idx);
        }
-        input_bits.iter().take(op_input_count.bits).for_each(|b| {


I am not quite sure why removing this is the right thing to do - it sounds plausible, I think I'm happy to trust you ;-).

Are there still calls to mark_bit_outdated elsewhere or can we remove it?

acl-cqc · 2026-03-17T11:04:26Z

tket/src/serialize/pytket/tests.rs

 }

+/// A simple circuit with some preset input and output bit registers,
+/// including multiple outputs for the same register.


Suggested change

/// including multiple outputs for the same register.

/// including multiple outputs of the same register.

??

tket/src/serialize/pytket/decoder.rs

acl-cqc · 2026-03-17T11:07:22Z

tket/src/serialize/pytket/extension/bool.rs

+            node,
+            hugr,
+            // Output bits use new registers, so don't reuse any input bits.
+            EmitCommandOptions::new().reuse_bits(|_| vec![]),


Yeah this looks like it might even have been the bigger fix?? Do we have a test that covers just this?

acl-cqc · 2026-03-17T11:08:35Z

tket/src/serialize/pytket/decoder/wires.rs

        // Convert the wire type, if needed.
        let wire_data = &self.wires[&wire];
-        let new_wire = config.transform_typed_value(wire, wire_data.ty(), ty, builder)?;
+        let found_wire_type = wire_data.ty();


not clear why you've changed this, you're not using found_wire_type anywhere you weren't before AFAICS. If you're intending the name to be an improvement then you should probably rename wire_data to found_wire instead?

aborgna-q requested a review from a team as a code owner March 13, 2026 14:59

aborgna-q requested a review from acl-cqc March 13, 2026 14:59

aborgna-q commented Mar 13, 2026

View reviewed changes

tket/src/serialize/pytket/decoder.rs Show resolved Hide resolved

aborgna-q commented Mar 13, 2026

View reviewed changes

aborgna-q added 2 commits March 17, 2026 10:42

Add failing test

5bce981

fix: Panic on repeated registers in pytket decoded output

c8ea134

aborgna-q force-pushed the ab/decoder-fixes branch from 204bbf7 to c8ea134 Compare March 17, 2026 10:42

acl-cqc approved these changes Mar 17, 2026

View reviewed changes

	/// including multiple outputs for the same register.
	/// including multiple outputs of the same register.

Conversation

aborgna-q commented Mar 13, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 13, 2026 •

edited

Loading